-
-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement api/tournament
endpoints
#50
Implement api/tournament
endpoints
#50
Conversation
Hey, thanks for the PR, since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the typos!
Re: return type annotations, ideally, we'd like to have accurate return types using TypedDicts
for new endpoints. See #38 for an example. Although I believe for tournaments, the return types are in some cases very complex and not 100% correct on the wiki, in which case it might be ok to leave them out. But best to check and we'd at least still want something like Dict[str, Any]
.
Also, for endpoints like withdraw
that only return "ok", it's better to not return anything (but they should still get a -> None
annotation). The result in this case will always be "ok" since it will throw if it errors, so returning something gives the incorrect impression that the return value has real meaning and should be checked and can also obscure the fact that errors will throw.
caffb3b
to
efef654
Compare
berserk/clients/tournaments.py
Outdated
} | ||
self._r.post(path=path, params=params, converter=models.Tournament.convert) | ||
|
||
def get_team_standings(self, tournament_id: str) -> Dict[str, Any]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the response is simple enough to be typed: https://lichess.org/api#tag/Arena-tournaments/operation/resultsByTournament please also include a test for it
efef654
to
986effa
Compare
Update - I am having trouble making that test pass due to my Python version being wrong and my local being weird. I'm gonna be busy at work again so may not have time to get to it. Please fix it if you can 🙏 but the other stuff should work. |
d5cf59c
to
5dcba7c
Compare
Thanks again for the numerous PRs! Quite a significant leap in coverage |
Description
get_team_standings
=/api/tournament/{id}/teams
(there is no/api/swiss
equivalent)update_team_battle
=/api/tournament/team-battle/{id}
(there is no/api/swiss
equivalent)join_arena
=/api/tournament/{id}/join
terminate_arena
=/api/tournament/{id}/terminate
withdraw_arena
=/api/tournament/{id}/withdraw
get_tournament
paramsid
uppercaseAfter merge
Please update #6 and check off all the five
api/tournament/...
endpoints in the issue description.Also there are several endpoints in the issue description that have already been implemented but not been checked off. I think there's only 6-7 endpoints left in that list and everything else can be checked off.